Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dbe7b45c02
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
This PR adds multi-battle support for Stations end-to-end (DB context detection → API/model aggregation → GraphQL schema/queries → UI rendering), with an additional client filter to optionally include upcoming battles.
Changes:
- Extend the Station GraphQL/API shape to include a
battlesarray (StationBattle) and plumb it through client queries/types. - Update Station map marker, tile memoization, and popup UI to handle multiple battles and a “Include Upcoming” filter.
- Update station search results timing to account for
battle_start(pre-start vs active).
Reviewed changes
Copilot reviewed 17 out of 19 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/services/queries/station.js | Fetches battles { ... } for station details. |
| src/services/queries/search.js | Adds battle_start to search results query. |
| src/features/station/useStationMarker.js | Marker icon logic updated to accept a selected battle. |
| src/features/station/StationTile.jsx | Computes battle-derived timers/state and memoizes based on battles. |
| src/features/station/StationPopup.jsx | Renders multiple battles in the popup; integrates filtering/visibility. |
| src/features/station/battleState.js | New shared client helper for selecting/ordering battles + refresh timestamps. |
| src/features/search/renderOption.jsx | Search option timer updated to consider battle_start. |
| src/features/search/OptionImage.jsx | Simplifies memoization for option images. |
| src/features/drawer/Stations.jsx | Adds “Include Upcoming” toggle to station filters UI. |
| server/src/services/DbManager.js | Detects station_battle table support via schema check (hasMultiBattles). |
| server/src/models/Station.js | Implements multi-battle joins/aggregation, filtering, and search support. |
| server/src/graphql/typeDefs/scanner.graphql | Adds battles: [StationBattle] and defines StationBattle. |
| server/src/graphql/typeDefs/map.graphql | Adds battle_start to Search type. |
| server/src/filters/builder/base.js | Adds default filter field includeUpcoming when permitted. |
| packages/types/lib/server.d.ts | Adds hasMultiBattles to DB context type. |
| packages/types/lib/scanner.d.ts | Adds StationBattle + Station.battles typings. |
| packages/locales/lib/human/en.json | Adds include_upcoming label. |
| config/default.json | Adds default stations.includeUpcoming: true. |
| config/custom-environment-variables.json | Adds env override mapping for default includeUpcoming. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const [now, setNow] = React.useState(() => Math.floor(Date.now() / 1000)) | ||
| React.useEffect(() => { | ||
| const interval = setInterval(() => { | ||
| setNow(Math.floor(Date.now() / 1000)) | ||
| }, 1000) | ||
| return () => clearInterval(interval) | ||
| }, []) | ||
| const target = startTime > now ? startTime : expireTime |
There was a problem hiding this comment.
Timer sets up its own 1s setInterval to update now, but it also calls useRelativeTimer, which already creates its own 1s interval. In the search dropdown this can create 2 intervals per rendered option row, which is a measurable performance/CPU hit.
Consider avoiding the extra interval by scheduling a single setTimeout for the start->end transition (only when startTime is in the future), or by enhancing useRelativeTimer/a shared hook to handle { startTime, expireTime } switching internally with one ticker.
| const [now, setNow] = React.useState(() => Math.floor(Date.now() / 1000)) | |
| React.useEffect(() => { | |
| const interval = setInterval(() => { | |
| setNow(Math.floor(Date.now() / 1000)) | |
| }, 1000) | |
| return () => clearInterval(interval) | |
| }, []) | |
| const target = startTime > now ? startTime : expireTime | |
| const [target, setTarget] = React.useState(() => | |
| startTime > Math.floor(Date.now() / 1000) ? startTime : expireTime, | |
| ) | |
| React.useEffect(() => { | |
| const now = Math.floor(Date.now() / 1000) | |
| if (startTime > now) { | |
| setTarget(startTime) | |
| const timeout = setTimeout(() => { | |
| setTarget(expireTime) | |
| }, Math.max(startTime * 1000 - Date.now(), 0)) | |
| return () => clearTimeout(timeout) | |
| } | |
| setTarget(expireTime) | |
| }, [expireTime, startTime]) |
| <StationBattleTimer | ||
| epoch={battleEndEpoch} | ||
| updated={updated} | ||
| countdown={showLiveCountdown} | ||
| hidden={hidden} |
There was a problem hiding this comment.
Upcoming battles (where battle_start is in the future) currently won’t show a start-time countdown: countdown is only enabled for isBattleActive, and the timer is always driven by battleEndEpoch. This means upcoming battles render an ends time with no countdown.
Consider driving the timer with battleStartEpoch (label starts) while battleStartEpoch > nowSeconds, then switch to battleEndEpoch once active.
| <Typography variant="subtitle1" align="center" color={textColor}> | ||
| {t('ends')}: {timeFormatter.format(new Date(target))} | ||
| </Typography> | ||
| {countdown ? ( | ||
| <Typography variant="h6" align="center" color={textColor}> | ||
| {relativeTime} | ||
| </Typography> |
There was a problem hiding this comment.
StationBattleTimer always labels the timestamp as ends, even when it’s being used for non-active/hidden battles. If you implement upcoming-battle countdowns (to battle_start), this component will also need a way to render starts vs ends (e.g., a label/start prop).
Requires UnownHash/Golbat#353.